[TableGen] Simplify generated code for isSubclass #117351
Conversation
Implement isSubclass with some tables and a single call to lower_bound instead of nested switches. Part of the motivation for this is improving compile time when clang-18 is used as a host compiler, since it seems to have trouble with very large switch statements.
|
@llvm/pr-subscribers-tablegen Author: Jay Foad (jayfoad) ChangesImplement isSubclass with some tables and a single call to lower_bound Part of the motivation for this is improving compile time when clang-18 Full diff: https://github.com/llvm/llvm-project/pull/117351.diff 1 Files Affected:
diff --git a/llvm/utils/TableGen/AsmMatcherEmitter.cpp b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
index ade393c11b7a24..68e9b434312a80 100644
--- a/llvm/utils/TableGen/AsmMatcherEmitter.cpp
+++ b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
@@ -2541,7 +2541,6 @@ static void emitIsSubclass(CodeGenTarget &Target,
OS << " if (A == B)\n";
OS << " return true;\n\n";
- bool EmittedSwitch = false;
for (const auto &A : Infos) {
std::vector<StringRef> SuperClasses;
if (A.IsOptional)
@@ -2551,42 +2550,29 @@ static void emitIsSubclass(CodeGenTarget &Target,
SuperClasses.push_back(B.Name);
}
- if (SuperClasses.empty())
+ if (SuperClasses.empty()) {
+ OS << " static constexpr ArrayRef<uint16_t> " << A.Name << "_SuperClasses;\n";
continue;
-
- // If this is the first SuperClass, emit the switch header.
- if (!EmittedSwitch) {
- OS << " switch (A) {\n";
- OS << " default:\n";
- OS << " return false;\n";
- EmittedSwitch = true;
}
- OS << "\n case " << A.Name << ":\n";
-
- if (SuperClasses.size() == 1) {
- OS << " return B == " << SuperClasses.back() << ";\n";
- continue;
- }
-
- if (!SuperClasses.empty()) {
- OS << " switch (B) {\n";
- OS << " default: return false;\n";
- for (StringRef SC : SuperClasses)
- OS << " case " << SC << ": return true;\n";
- OS << " }\n";
- } else {
- // No case statement to emit
- OS << " return false;\n";
- }
+ OS << " static constexpr uint16_t " << A.Name << "_SuperClasses[] = {";
+ ListSeparator LS;
+ for (auto &SC : SuperClasses)
+ OS << LS << SC;
+ OS << "};\n";
}
+ OS << "\n";
- // If there were case statements emitted into the string stream write the
- // default.
- if (EmittedSwitch)
- OS << " }\n";
- else
- OS << " return false;\n";
+ OS << " static constexpr ArrayRef<uint16_t> SuperClassTable[] = {\n";
+ OS << " {}, // InvalidMatchClass\n";
+ OS << " {}, // OptionalMatchClass\n";
+ for (const auto &A : Infos)
+ OS << " " << A.Name << "_SuperClasses,\n";
+ OS << " };\n\n";
+
+ OS << " ArrayRef<uint16_t> SuperClasses = SuperClassTable[(unsigned)A];\n";
+ OS << " const uint16_t *It = lower_bound(SuperClasses, (unsigned)B);\n";
+ OS << " return It != SuperClasses.end() && *It == (unsigned)B;\n";
OS << "}\n\n";
}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
IIUC we could have two nested switches (constant time lookup), but now we are using binary search (logarithmic time lookup). Will this cause any time related regressions? |
Good question. I tried measuring this with |
I think I'm seeing around 1% slowdown in some cases, which is not great. Part of the difference is that isSubclass is now simple enough to be inlined, which might affect performance regardless of what algorithm is used inside isSubclass. Here's an example I used for testing: perf stat -e cycles -r 300 llvm-mc -triple x86_64-unknown-unknown test/MC/X86/I86-64.s -filetype=null |
Implement isSubclass with some tables and a single call to lower_bound
instead of nested switches.
Part of the motivation for this is improving compile time when clang-18
is used as a host compiler, since it seems to have trouble with very
large switch statements.